Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add enum RawKind and is_* to RawValue #1223

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add enum RawKind and is_* to RawValue #1223

wants to merge 1 commit into from

Conversation

bheylin
Copy link
Contributor

@bheylin bheylin commented Dec 11, 2024 •
edited
Loading

I mentioned the proc-macro based JSON processor I'm using in PR #1221. This processor ultimately converts the JSON to a Deserializable Rust struct. While converting, it has specialized null handling and custom Errors for missing fields and such.

To accomplish this I currently use an extension trait to provide methods that report what JSON kind (null, bool, string, etc) the RawValue contains and a set of tests to assert the invariants of a RawValue. Invariants such as:

  • A RawValue has whitespace stripped during deserialization
  • An empty string can't be deserialized into a RawValue.

I think knowing the type of a RawValue can be useful to all users of serde_json.

What do you think?

@bheylin bheylin force-pushed the add-type-inspection-fns-to-raw-value branch from 8018a9a to d09c5fa Compare December 11, 2024 21:36
@Marcono1234
Copy link

Value has similar is_... methods as well. Maybe there could be a common ValueKind (or similar) used for both, instead of adding this specific RawKind (and risking that it needs to be duplicated / renamed in the future)?

(This is just an idea from a drive-by user though; would be good to wait for the opinion of the maintainer before acting on this.)

@bheylin
Copy link
Contributor Author

bheylin commented Jan 5, 2025 •
edited
Loading

Value has similar is_... methods as well. Maybe there could be a common ValueKind (or similar) used for both, instead of adding this specific RawKind (and risking that it needs to be duplicated / renamed in the future)?

(This is just an idea from a drive-by user though; would be good to wait for the opinion of the maintainer before acting on this.)

Hi @Marcono1234, thanks for your comment.

I don't see the benefit in merging the Value and RawKind. Besides the similar variants, they have a fundamentally different purpose. Value stores the value of the field while RawKind simply states what kind of value is stored in the field.

Merging the Value and RawKind enums would result in breaking changes in the much used Value enum and add complexity to the Value type that isn't justified.

(and risking that it needs to be duplicated / renamed in the future)?

Why do you think RawKind would need to be duplicated or renamed in the future?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bheylin @Marcono1234